-
-
Notifications
You must be signed in to change notification settings - Fork 4k
fix(server): bump sharp to 0.34.3 + add unlimited
flag
#20327
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@@ -141,6 +141,7 @@ export class MediaRepository { | |||
failOn: options.processInvalidImages ? 'none' : 'error', | |||
limitInputPixels: false, | |||
raw: options.raw, | |||
unlimited: true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should presumably be guarded behind some sort of configuration option (e.g. an environment variable).
Enabling this should be carefully considered in deployments where trust in users is weaker.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you elaborate on the potential issues or security concerns?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could presumably create a malicious image which consumes enough memory to crash the server
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps it'd be better to make limitInputPixels
configurable? That sounds like it might also solve this and be a safer approach. Potentially it could be set based on available system memory, which might also prevent people from hitting this as often
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could presumably create a malicious image which consumes enough memory to crash the server
Thank you, this is what I meant
the base-image is merged |
e9d4a34
to
a135c22
Compare
856a5dd
to
50715d6
Compare
0.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We use pnpm now, so this PR needs to be updated to bump sharp using pnpm.
@@ -1,5 +1,5 @@ | |||
# dev build | |||
FROM ghcr.io/immich-app/base-server-dev:202507291116@sha256:e38543bdd77a02ed156cd9175ed11e9c16dccf48c418d46ecda48ce684de456a AS dev | |||
FROM ghcr.io/immich-app/base-server-dev:commit-e54882f68fed72fa84c0bb3c422e374a7c082703 AS dev |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
libvips 8.17 is already in the current base image, so no need to change the tag.
Superseded by #21596 |
Description
This bumps
sharp
to0.34.3
and adds theunlimited
flag to disable all memory-related restrictions.Fixes #20028
To Do
I applied the changes locally in order to test the changes in this PR.
How Has This Been Tested?
Checklist:
src/services/
uses repositories implementations for database calls, filesystem operations, etc.src/repositories/
is pretty basic/simple and does not have any immich specific logic (that belongs insrc/services/
)